unix: fix reopened fd bug #971
Conversation
|
Here's a comment for you: where's the test case? :-) |
|
Haha, actually this one would be quite complicated. But I'll do my best. |
When fd is closed and new one (with the same number) is opened inside kqueue/epoll/port loop's callback - stale events might invoke callbacks on wrong watchers. Check if watcher was changed after invocation and invalidate all events with the same fd. fix #826
|
@bnoordhuis added test. |
| unsigned int nwatchers; | ||
| unsigned int i; | ||
|
|
||
| if (len <= loop->nwatchers) | ||
| return; | ||
|
|
||
| nwatchers = next_power_of_two(len); | ||
| watchers = realloc(loop->watchers, nwatchers * sizeof(loop->watchers[0])); | ||
| nwatchers = next_power_of_two(len + 2) - 2; |
There was a problem hiding this comment.
This logic is unsound. Say that len == 14. In that case, next_power_of_two(14 + 2) - 2 is still 14.
There was a problem hiding this comment.
Yep, and that's ok. Since I'm allocating nwatchers + 2 below.
|
@bnoordhuis fixed all mentioned nits, except conversion to |
|
And thank you :) |
|
Fixed all nits, again :) |
|
@bnoordhuis time to review it? :) |
| /* Copy watchers, preserving fake one in the end */ | ||
| if (loop->watchers == NULL) { | ||
| fake_watcher_list = watchers[loop->nwatchers]; | ||
| fake_watcher_count = watchers[loop->nwatchers + 1]; |
There was a problem hiding this comment.
Using loop->watchers here would be a little clearer.
| ASSERT(status == 0); | ||
| ASSERT(connect_reqs <= req && | ||
| req <= (connect_reqs + ARRAY_SIZE(connect_reqs))); | ||
| i = (unsigned int) (req - connect_reqs) / sizeof(*req); |
There was a problem hiding this comment.
Just req - connect_reqs?
|
Few comments but mostly LGTM. A possible future optimization is to store the current index in loop->watchers so you only have to scan (nfds - i) entries instead of nfds entries. |
When fd is closed and new one (with the same number) is opened inside
kqueue/epoll/port loop's callback - stale events might invoke callbacks
on wrong watchers.
Check if watcher was changed after invocation and invalidate all events
with the same fd.
fix #826
/cc @bnoordhuis